-
Notifications
You must be signed in to change notification settings - Fork 17
fix(contracts): optimize test fixture deployment speed #697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Dramatically improved contract test performance from 47 seconds to 2 seconds per fixture (22.7x speedup, 96% reduction in deployment time). Root cause was inefficient batch processing during BLR configuration creation in test environments. Performance Impact: - Infrastructure fixture: 47s to 2s (95.7% faster) - Full test suite runtime significantly reduced - No performance degradation on real networks Root Causes Fixed: 1. Inefficient Batch Size in Tests (22s delay per config) - Fixture was using batchSize=2 instead of DEFAULT_BATCH_SIZE (15) - Created 23 batches instead of 3-4 batches per configuration - With 1000ms delays between batches: 23 × 1000ms = 22s per config - Two configurations (equity + bond) = 44s total unnecessary delay 2. Unnecessary Network Delays on Instant-Mining Networks - 1000ms delays designed for real networks (prevent RPC overload) - Hardhat/local networks process transactions instantly - Each deployment: 44 delays × 1000ms = 44 seconds of waste Solutions Implemented: 1. Fixed Test Fixture Batch Size (infrastructure.fixture.ts) - Changed default from batchSize=2 to DEFAULT_BATCH_SIZE (15) - Reduces batches from 23 to 3-4 per configuration - Import DEFAULT_BATCH_SIZE from scripts for consistency 2. Network-Aware Batch Processing (blrConfigurations.ts) - Created isInstantMiningNetwork() to detect instant networks - Skip 1000ms delays on hardhat and local networks - Optimized batch size: single batch (capped at 20) on instant - Real networks unchanged: still use batching with delays 3. Network Detection Helper (networkConfig.ts) - Added isInstantMiningNetwork(): hardhat || local - Clear distinction: instant (hardhat, local) vs real (hedera) - hedera-local correctly treated as real network - Deprecated isLocalNetwork() in favor of clearer naming 4. Confirmations Parameter Propagation - Added confirmations to createEquityConfiguration() - Added confirmations to createBondConfiguration() - Test environments use 0 confirmations (instant) - Proper parameter flow: fixture → modules → operations Additional Improvements: 5. Refactored ERC20Permit Test (erc20Permit.test.ts) - Replaced manual timestamp retrieval with getDltTimestamp() - 8 instances updated for cleaner, maintainable test code 6. Preserved Original deploymentTime Tracking - Restored deploymentTime field in DeploymentOutput type - Tracks total workflow execution time - Added to checkpoint conversion and test assertions Files Modified: Infrastructure & Operations: - scripts/infrastructure/networkConfig.ts - scripts/infrastructure/operations/blrConfigurations.ts - scripts/infrastructure/checkpoint/utils.ts - scripts/infrastructure/checkpoint/NullCheckpointManager.ts - scripts/infrastructure/index.ts Domain Modules: - scripts/domain/equity/createConfiguration.ts - scripts/domain/bond/createConfiguration.ts - scripts/index.ts Workflows: - scripts/workflows/deploySystemWithNewBlr.ts - scripts/workflows/deploySystemWithExistingBlr.ts CLI Scripts: - scripts/cli/hardhat.ts (preserved) - scripts/cli/standalone.ts (preserved) Tests: - test/fixtures/infrastructure.fixture.ts - test/contracts/unit/layer_1/ERC1400/ERC20Permit/erc20Permit.test.ts Technical Notes: - Gas Limit Safety: Single-batch optimization capped at 20 facets - Backward Compatibility: Real networks unchanged - Test Environment Detection: Uses hre.network.name - Type Safety: All TypeScript types properly updated Verification: Before: npx hardhat test infrastructure.fixture.ts # 47 seconds After: npx hardhat test infrastructure.fixture.ts # 2 seconds No functionality changes - pure performance optimization. Signed-off-by: Miguel_LZPF <[email protected]>
Signed-off-by: Axel-IoBuilders <[email protected]> Signed-off-by: Miguel_LZPF <[email protected]>
2e7d438 to
94e9c7f
Compare
The nominal value decimals feature (PR #693) was cherry-picked into this branch but the mass-payout test mocks were not updated. This caused struct size mismatches between the test asset mock implementations and the actual contract interfaces. Changes: - IAssetMock.sol: Added `uint8 nominalValueDecimals` field to both EquityDetailsData and BondDetailsData structs to match ATS contract interface definitions - AssetMock.sol: Updated getBondDetails() to set nominalValueDecimals = 2 - AssetMock.sol: Implemented getEquityDetails() with proper struct initialization including all equity rights flags, currency, nominal value, and nominalValueDecimals Impact: All 131 tests now pass. Test fixtures correctly represent contract state without struct size mismatches. Signed-off-by: Miguel_LZPF <[email protected]>
2e32269 to
8fe120d
Compare
AlbertoMolinaIoBuilders
approved these changes
Nov 17, 2025
themariofrancia
pushed a commit
that referenced
this pull request
Dec 9, 2025
Signed-off-by: Miguel_LZPF <[email protected]> Signed-off-by: Axel-IoBuilders <[email protected]>
themariofrancia
pushed a commit
that referenced
this pull request
Dec 10, 2025
Signed-off-by: Miguel_LZPF <[email protected]> Signed-off-by: Axel-IoBuilders <[email protected]>
themariofrancia
pushed a commit
that referenced
this pull request
Dec 10, 2025
Signed-off-by: Miguel_LZPF <[email protected]> Signed-off-by: Axel-IoBuilders <[email protected]> Signed-off-by: Mario Francia <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Root Causes Fixed
1. Inefficient Batch Size in Tests (22s delay per config)
batchSize=2instead ofDEFAULT_BATCH_SIZE(15)2. Unnecessary Network Delays on Instant-Mining Networks
Solutions Implemented
Network-Aware Batch Processing
isInstantMiningNetwork()helper (hardhat, local)Test Fixture Optimization
Additional Improvements
getDltTimestamp()helper (8 instances)deploymentTimefield in DeploymentOutput typePerformance Impact
Result: 96% reduction in test deployment time, no functionality changes
Backward Compatibility
Changeset
✅ Changeset included:
.changeset/test-performance-optimization.md